feat(subscriptions): foundation + read methods (PR 4/7)#516
Conversation
|
3 raw-string enum violations (CLAUDE.md: use enums for fixed value sets — NEVER leave raw strings/numbers):
Everything else looks good — architecture, pagination pattern, transform pipeline, @track decorators, JSDoc structure, test coverage, and integration test setup all follow conventions. |
Three CLAUDE.md violations (use enums for fixed value sets — NEVER raw strings): - subscriptions.models.ts JSDoc example: 'Slack' → NotificationMode.Slack (with import for self-contained example) - subscriptions.ts JSDoc example: same fix - subscriptions.test.ts assertion: 'InApp' → NotificationMode.InApp (with import) Addresses review comments on PR #516. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
All findings addressed in the latest commit on this branch. Inline reply threads are resolved with commit references. |
|
1 new finding this run: subscriptions.ts:33-34 — PR-description sentence in the class-level JSDoc ("This PR ships the three read methods…") will surface in IDE tooltips for SDK consumers and become stale once mutation methods land in the follow-up PR. Suggestion posted inline to drop the last paragraph. |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
fe38d8a to
28f2e06
Compare
2f0d37c to
1d382c9
Compare
|
1 new finding this run: tests/integration/shared/notification/subscriptions.integration.test.ts:8 — |
|
| headers: createHeaders({ [TENANT_ID]: tenantId }), | ||
| ...(options?.publishers ? { params: { Publishers: options.publishers } } : {}), |
There was a problem hiding this comment.
Please test getAll() with multiple publishers as this is the first method in the SDK that passes an array as a query param. The sample output in the PR description only covers publishers: ['Apps'] a single-element array
There was a problem hiding this comment.
I didn't get your point. Passing single element or multiple element API logic doesn't change.
The API is already well tested by NS integration/e2e test.
There was a problem hiding this comment.
The concern is not about API, In SDK im not sure if array in parameters is handled correctly, im worried if this array is correctly passed to API
| export interface SubscriptionGetResponse { | ||
| /** Publishers with their topics and subscription state. */ | ||
| publishers: SubscriptionPublisher[]; | ||
| } |
There was a problem hiding this comment.
Why are these responses wrapped? Lets not wrap the responses
There was a problem hiding this comment.
We have the same pattern in our API contract. It's done to keep both DTOs aligned.
Think of a scenario where we want to add a field to this DTO in addition to the publishers. We can simply add it in both places without breaking compatibility.
It's better to keep them aligned.
Also, what benefit do you see in unwrapping them?
There was a problem hiding this comment.
| export interface SubscriptionGetPublishersResponse { | ||
| /** Publishers with their topic catalogue. */ | ||
| publishers: SubscriptionPublisherBase[]; | ||
| } |
There was a problem hiding this comment.
Same here and for below method as well
…ods [internal] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1d382c9 to
18c7fcb
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |



PR 4/7 in the Notification SDK stack. Rebased onto
main(foundation #512, mark-read #513, and delete #514 are now merged); this branch is a single squashed commit.Adds the
Subscriptionsservice alongside the existingNotificationsservice. Both live under@uipath/uipath-typescript/notificationsas discrete classes (no shared state). This PR ships the 3 read methods + subscription model foundation.All three methods are tagged
@internal(theNotificationServicescope is internal), so nodocs/oauth-scopes.mdormkdocs.ymlnav entries are added — matching #512–#514.Methods Added
subscriptions.getAll()getAll(tenantId: string, options?: SubscriptionGetAllOptions): Promise<SubscriptionGetResponse>subscriptions.getPublishers()getPublishers(tenantId: string, options?: SubscriptionGetPublishersOptions): Promise<SubscriptionGetPublishersResponse>subscriptions.getSupportedChannels()getSupportedChannels(tenantId: string): Promise<SubscriptionGetSupportedChannelsResponse>Endpoints Called
subscriptions.getAll()notificationservice_/usersubscriptionservice/api/v1/UserSubscriptionsubscriptions.getPublishers()notificationservice_/usersubscriptionservice/api/v1/UserSubscription/GetPublisherssubscriptions.getSupportedChannels()notificationservice_/usersubscriptionservice/api/v1/UserSubscription/GetSupportedChannelStatusResponse Modeling
getAll()andgetPublishers()populate genuinely different field sets (verified against the backendGetUserSubscriptionHandlersvsGetPublisherDetailsHandler), so each has its own explicit response type:getAll()→SubscriptionGetResponse— full per-publisher, per-topic, per-channel subscription state (SubscriptionPublisher[]).getPublishers()→SubscriptionGetPublishersResponse— discovery-only catalogue (SubscriptionPublisherBase[]) carrying only identity fields + topic list, no subscription state.SubscriptionPublisher/SubscriptionTopicextend the base shapesSubscriptionPublisherBase/SubscriptionTopicBase, so the discovery fields are declared once. Accessing state-only fields (isSubscribed,modes,isUserOptin, …) on agetPublishers()result is a compile error, steering callers togetAll().getSupportedChannels()returns Email/Slack/Teams withisEnabled.InAppis implicitly always available and not included in the response.NotificationModedelivery-channel enum (InApp/Email/Slack/Teams) tonotifications.types.ts, shared by the subscription channel/mode shapes.@track(...)telemetry decorators and forward the actingtenantIdas the first argument.Example Usage
Sample SDK Responses
subscriptions.getSupportedChannels(tenantId){ "channels": [ { "name": "Email", "isEnabled": true }, { "name": "Slack", "isEnabled": false }, { "name": "Teams", "isEnabled": false } ] }subscriptions.getPublishers(tenantId, { name: 'Apps' })—SubscriptionGetPublishersResponse{ "publishers": [ { "id": "3b5c8128-8af0-46ec-eb1d-08da31909e0f", "name": "Apps", "displayName": "Apps", "topics": [ { "id": "b3f9b2fd-122f-4d40-3d88-08da31909e1a", "name": "Apps.Shared", "displayName": "Apps Shared", "description": "Apps is Shared", "group": "Apps Activities" }, { "id": "7a647eb5-381e-4712-55de-08db6be7681e", "name": "Apps.Cloned", "displayName": "App Duplicated", "description": "App is Duplicated", "group": "Apps Activities" } ] } ] }The discovery endpoint returns only
id/name/displayName/description/groupon each topic (theSubscriptionTopicBaseshape).subscriptions.getAll(tenantId, { publishers: ['Apps'] })—SubscriptionGetResponse{ "publishers": [ { "id": "3b5c8128-8af0-46ec-eb1d-08da31909e0f", "name": "Apps", "displayName": "Apps", "isUserOptin": true, "modes": [ { "name": "InApp", "isActive": true }, { "name": "Email", "isActive": true } ], "topics": [ { "id": "b3f9b2fd-122f-4d40-3d88-08da31909e1a", "name": "Apps.Shared", "category": "Success", "isSubscribed": true, "isMandatory": false, "modes": [ { "name": "InApp", "isSubscribed": true, "isSubscribedByDefault": true }, { "name": "Email", "isSubscribed": true, "isSubscribedByDefault": true } ] } ] } ] }Verification
npm run typechecknpm run lintnpm run test:unitnpm run buildFiles
src/utils/constants/endpoints/notification.ts(SUBSCRIPTION_ENDPOINTS,SUBSCRIPTION_API_BASE)src/models/notification/notifications.types.ts(NotificationMode)src/models/notification/subscriptions.types.ts(SubscriptionPublisherBase/SubscriptionPublisher,SubscriptionTopicBase/SubscriptionTopic,SubscriptionMode,AllowedMode,SupportedChannel,SubscriptionEntity, response + options types)src/models/notification/subscriptions.models.ts(SubscriptionServiceModel),src/models/notification/index.tssrc/services/notification/subscriptions.ts,src/services/notification/index.tstests/unit/services/notification/subscriptions.test.ts(8 tests)tests/integration/shared/notification/subscriptions.integration.test.tstests/integration/config/unified-setup.ts(Subscriptions registration)tests/utils/constants/notification.ts(subscription constants,ERROR_PUBLISHER_NOT_FOUND),tests/utils/mocks/notification.ts(createBasicSubscriptionPublisher/Base,createBasicSubscriptionTopic/Base,createBasicSupportedChannels)PR Stack
feat/notifications-sdkfeat/notifications-mark-readfeat/notifications-deletefeat/subscriptions-sdk(this PR)feat/subscriptions-topic-updatesfeat/subscriptions-publisher-updatesfeat/subscriptions-mode-reset🤖 Generated with Claude Code